Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix/choosing_apple_device #1753

Merged
merged 3 commits into from
Oct 17, 2024
Merged

fix/choosing_apple_device #1753

merged 3 commits into from
Oct 17, 2024

Conversation

ElenaDiachenko
Copy link
Contributor

Description

  • [ios, tvos] npx rnv run -p ios -d -t random installs app when 1 device is connected
  • [ios, tvos] npx rnv run -p ios -d -t doesn't ask to choose from device list when 1 device is available
  • [ios, tvos] npx rnv run -p ios -t asks only simulators

Related issues

Npm releases

n/a

@ElenaDiachenko ElenaDiachenko self-assigned this Oct 9, 2024
@ElenaDiachenko ElenaDiachenko added this to the 1.5 milestone Oct 9, 2024
Copy link
Collaborator

@GabrieleKaceviciute GabrieleKaceviciute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at least one device is active:

  • npx rnv run -p ios -t device name -> doesn't find device. Expected - install to specified device.
  • npx rnv run -p ios -t random -> asks from simulators. Expected - ask from simulators and active devices.
  • npx rnv run -p ios -d -t device IP -> asks from active device. Expected - install to specified device.
  • npx rnv run -p ios -t device IP -> asks from simulators. Expected - install to specified device.
  • npx rnv run -p ios -t random IP -> asks from simulators. Expected - ask from simulators and active devices.

Note: same on tvos.

const target = c.runtime.target?.replace(/(\s+)/g, '\\$1');

p = `--simulator ${target}`;
p = `--simulator ${target}`;

Check warning

Code scanning / CodeQL

Unsafe shell command constructed from library input Medium

This string concatenation which depends on
library input
is later used in a
shell command
.

Copilot Autofix AI 2 months ago

To fix the problem, we should avoid using string concatenation to construct the shell command. Instead, we can use child_process.execFile to safely pass the arguments to the command without risking shell injection. This approach ensures that the input is treated as an argument rather than part of the command string.

Suggested changeset 1
packages/sdk-apple/src/runner.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/sdk-apple/src/runner.ts b/packages/sdk-apple/src/runner.ts
--- a/packages/sdk-apple/src/runner.ts
+++ b/packages/sdk-apple/src/runner.ts
@@ -79,5 +79,5 @@
         if (selectedDevice.udid) {
-            p = `--udid ${selectedDevice.udid}`;
+            p = ['--udid', selectedDevice.udid];
         } else {
-            p = `--device ${selectedDevice.name}`;
+            p = ['--device', selectedDevice.name];
         }
@@ -217,5 +217,5 @@
         if (!desiredSim?.isDevice) {
-            const target = c.runtime.target?.replace(/(\s+)/g, '\\$1');
+            const target = c.runtime.target;
 
-            p = `--simulator ${target}`;
+            p = ['--simulator', target];
         } else {
EOF
@@ -79,5 +79,5 @@
if (selectedDevice.udid) {
p = `--udid ${selectedDevice.udid}`;
p = ['--udid', selectedDevice.udid];
} else {
p = `--device ${selectedDevice.name}`;
p = ['--device', selectedDevice.name];
}
@@ -217,5 +217,5 @@
if (!desiredSim?.isDevice) {
const target = c.runtime.target?.replace(/(\s+)/g, '\\$1');
const target = c.runtime.target;

p = `--simulator ${target}`;
p = ['--simulator', target];
} else {
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@Marius456 Marius456 added e2e and removed e2e labels Oct 14, 2024
@aurimasmi aurimasmi self-requested a review October 14, 2024 07:03
packages/sdk-apple/src/runner.ts Show resolved Hide resolved
@Marius456 Marius456 added e2e and removed e2e labels Oct 16, 2024
@pauliusguzas pauliusguzas merged commit ef8f440 into main Oct 17, 2024
12 of 13 checks passed
@Marius456 Marius456 deleted the fix/choosing_apple_device branch December 16, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants